Skip to content

Remove unreachable error constructors#1211

Merged
Jimbo4350 merged 2 commits into
masterfrom
jordan/remove-unused-error-constructors
May 18, 2026
Merged

Remove unreachable error constructors#1211
Jimbo4350 merged 2 commits into
masterfrom
jordan/remove-unused-error-constructors

Conversation

@Jimbo4350
Copy link
Copy Markdown
Contributor

@Jimbo4350 Jimbo4350 commented May 15, 2026

Changelog

- description: |
    Remove unreachable error constructors. None of these were produced
    anywhere in the repo — only defined, pretty-printed, and listed in
    golden fixtures.

    Removed:

    - `TransactionValidityError` (entire type, both constructors:
      `TransactionValidityIntervalError`,
      `TransactionValidityCostModelError`).
    - `TxFeeEstimationTransactionTranslationError`, the
      `TxFeeEstimationError` constructor that wrapped the now-removed
      `TransactionValidityError`.
    - `TxBodyErrorByronEraNotSupported`,
      `TxBodyErrorMissingParamMinUTxO`, and
      `TxBodyErrorNonAdaAssetsUnbalanced` from the legacy
      `Cardano.Api.Tx.Internal.Fee.TxBodyErrorAutoBalance`.
    - `TxBodyErrorNonAdaAssetsUnbalanced` from the experimental
      `Cardano.Api.Experimental.Tx.Internal.Fee.TxBodyErrorAutoBalance`.

    The corresponding `test_TransactionValidityError` golden test, the
    three dead `TxBodyErrorAutoBalance` fixture entries, and their
    expected `.txt` files are dropped too. The
    `Ouroboros.Consensus.HardFork.History` import in `Fee.hs` becomes
    unused and is removed.
  type:
   - breaking
  projects:
   - cardano-api

Context

Continuing the pattern of PR #1180 / commit 926606fdd ("Remove unused TxBodyProtocolParamsConversionError constructor"). An audit of the TxBodyError / TxBodyErrorAutoBalance / TxFeeEstimationError / TransactionValidityError types found six error constructors with no production producers — only the data declaration, the prettyError pattern match, and golden fixtures referenced them.

Construction-site search (across cardano-api/src, cardano-api/test, cardano-api/gen) found zero call sites of the form Left X … / X … / first X … / \$ X … for any of the constructors removed here.

Since both TransactionValidityError constructors are dead, the whole GADT is removed, which in turn makes TxFeeEstimationTransactionTranslationError unreachable.

How to trust this PR

cabal build cardano-api -j4
cabal test cardano-api-golden     # 121 / 121 pass

Sanity-check that no producer remains for any removed constructor:

git grep -nE 'TransactionValidity(Interval|CostModel)Error|TxFeeEstimationTransactionTranslationError|TxBodyErrorByronEraNotSupported|TxBodyErrorMissingParamMinUTxO|TxBodyErrorNonAdaAssetsUnbalanced'

After this PR the only matches should be in CHANGELOG.md / release notes.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Self-reviewed the diff
  • Changelog fragment added in .changes/ (added in a follow-up commit once PR number is known, matching the project's existing pattern)

@Jimbo4350
Remove unreachable error constructors
1c005b5 
None of these were produced anywhere in the repo — only defined,
pretty-printed, and listed in golden fixtures.

Removed:
  - TransactionValidityError (entire type, both constructors:
    TransactionValidityIntervalError, TransactionValidityCostModelError)
  - TxFeeEstimationTransactionTranslationError, the TxFeeEstimationError
    constructor that wrapped TransactionValidityError
  - TxBodyErrorByronEraNotSupported, TxBodyErrorMissingParamMinUTxO, and
    TxBodyErrorNonAdaAssetsUnbalanced from the legacy
    Cardano.Api.Tx.Internal.Fee.TxBodyErrorAutoBalance
  - TxBodyErrorNonAdaAssetsUnbalanced from the experimental
    Cardano.Api.Experimental.Tx.Internal.Fee.TxBodyErrorAutoBalance

The corresponding test_TransactionValidityError golden test, the three
dead TxBodyErrorAutoBalance fixture entries, and their expected .txt
files are dropped too. The Ouroboros.Consensus.HardFork.History import
in Fee.hs becomes unused and is removed.
Copilot AI review requested due to automatic review settings May 15, 2026 19:30
Copilot AI reviewed May 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes six error constructors from cardano-api's transaction/fee error types that have no producers anywhere in the repo (only definition, pretty-printing, and golden fixtures referenced them). It continues the cleanup pattern from PR #1180. Because both constructors of TransactionValidityError are dead, the entire GADT is dropped, which in turn makes the wrapping TxFeeEstimationTransactionTranslationError unreachable and is also removed.

Changes:

  • Drop TransactionValidityError (and its export from Cardano.Api.Tx/Fee), the wrapping TxFeeEstimationTransactionTranslationError, and four dead TxBodyErrorAutoBalance constructors across legacy and experimental modules.
  • Remove the corresponding test_TransactionValidityError golden test, three dead TxBodyErrorAutoBalance fixture entries, and their .txt files.
  • Drop the now-unused Ouroboros.Consensus.HardFork.History import in Fee.hs and add a breaking Herald changelog fragment.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cardano-api/src/Cardano/Api/Tx/Internal/Fee.hs Removes TransactionValidityError GADT, TxFeeEstimationTransactionTranslationError, three dead TxBodyErrorAutoBalance constructors, and the now-unused Consensus import.
cardano-api/src/Cardano/Api/Tx.hs Drops the TransactionValidityError(..) re-export.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/Fee.hs Removes TxBodyErrorNonAdaAssetsUnbalanced and its pretty-printer.
cardano-api/test/cardano-api-golden/Test/Golden/ErrorsSpec.hs Drops test_TransactionValidityError and the three dead TxBodyErrorAutoBalance fixture entries.
cardano-api/test/cardano-api-golden/files/errors/.../TxBodyErrorByronEraNotSupported.txt Removes the dead golden fixture file.
cardano-api/test/cardano-api-golden/files/errors/.../TxBodyErrorMissingParamMinUTxO.txt Removes the dead golden fixture file.
cardano-api/test/cardano-api-golden/files/errors/.../TxBodyErrorNonAdaAssetsUnbalanced.txt Removes the dead golden fixture file.
cardano-api/test/cardano-api-golden/files/errors/.../TransactionValidityCostModelError.txt Removes the dead golden fixture file.
.changes/20260515_cardano_api_remove_unreachable_error_constructors.yml Adds a Herald changelog fragment correctly classified as breaking.

palas
palas approved these changes May 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@palas palas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid PR 👍

Only errors and golden tests over those errors were deleted, and it compiles, so I cannot think of any issue with that

@Jimbo4350 Jimbo4350 added this pull request to the merge queue May 18, 2026
Merged via the queue into master with commit 1b3393d May 18, 2026
31 checks passed
@Jimbo4350 Jimbo4350 deleted the jordan/remove-unused-error-constructors branch May 18, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@carbolymer carbolymer carbolymer approved these changes

@palas palas palas approved these changes

@erikd erikd Awaiting requested review from erikd erikd is a code owner

@CarlosLopezDeLara CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara CarlosLopezDeLara is a code owner

@disassembler disassembler Awaiting requested review from disassembler disassembler is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jimbo4350 @carbolymer @palas